fix(daemon): isolate disconnect cancellation and resource teardown by devin-ai-integration[bot] · Pull Request #1225 · callstack/agent-device

devin-ai-integration · 2026-07-12T10:19:38Z

Summary

Scopes disconnect cancellation and teardown ownership to the affected request/session, addressing #1220 without changing public wire contracts or Apple runner retention policy.

HTTP pre-header disconnects and socket disconnects abort only their registered request; concurrent duplicate supplied request IDs fail with structured INVALID_ARGS instead of replacing another request's controller.
Apple runner startup consumes the request signal across xctestrun build, launch, and initial readiness. Cancellation preserves the typed request_canceled error and guarantees a newly launched runner is disposed before persistence or invalidated while still unready, without affecting unrelated sessions.
Session teardown continues independent cleanup after failures and aggregates diagnostics. Targeted close preserves original platform/pre-close AppError details, never records a failed close as successful, and retains required runner-stop-before-close ordering for physical Apple targets.
Regressions cover server-observed pre-header cancellation, duplicate request-ID ownership, build/launch/readiness cancellation with lease/session isolation, teardown continuation, exact close errors, and pre-close ordering.

Validation on current main: check:affected --run (4,090 unit tests and 124 provider-integration tests), format, lint, typecheck, layering, Fallow, build, and focused cancellation suites.

Link to Devin session: https://app.devin.ai/sessions/b1c3093bf2b248ca82a1a5df89152d02
Requested by: @thymikee

devin-ai-integration · 2026-07-12T10:19:41Z

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

Disable automatic comment, CI, and merge conflict monitoring

github-actions · 2026-07-12T10:20:18Z

Size Report

Metric	Base	Current	Diff
JS raw	1.7 MB	1.7 MB	+1.5 kB
JS gzip	538.5 kB	539.1 kB	+524 B
npm tarball	648.2 kB	648.7 kB	+474 B
npm unpacked	2.3 MB	2.3 MB	+1.5 kB

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	27.9 ms	28.1 ms	+0.2 ms
CLI --help	56.9 ms	58.1 ms	+1.3 ms

Top changed chunks:

Chunk	Raw diff	Gzip diff
`dist/src/session.js`	+840 B	+262 B
`dist/src/runner-client.js`	+496 B	+208 B
`dist/src/internal/daemon.js`	+166 B	+60 B
`dist/src/apps.js`	+1 B	+2 B
`dist/src/selector-runtime.js`	0 B	-1 B

devin-ai-integration · 2026-07-12T10:37:04Z

P1: request-scoped cancellation still does not cover runner startup/build, so removing the global fallback can leave the exact long-running prep process it previously killed. runPrepareAttempt calls ensureRunnerSession(device, options) before any signal-aware transport; startRunnerSessionWithLease calls ensureXctestrunArtifact, whose xcodebuild build-for-testing uses runCmdStreaming without signal, and prep children are tracked only in the global runnerPrepProcesses Set. A disconnect now only aborts the request controller, so that build can continue until its normal timeout and consume/produce runner state after the client is gone. Please thread the request signal through runner session/artifact preparation (or add request-owned prep tracking and scoped abort), then add a test that disconnecting request A terminates A’s prep while request B’s prep/session remains alive. Do not restore a global abort.

thymikee · 2026-07-12T10:39:16Z

Re-review at 3f0f7c6 found two blockers:

P1 — request cancellation does not cover runner startup. executeRunnerCommand obtains the request signal, but ensureRunnerSession(...) at src/platforms/apple/core/runner/runner-lifecycle.ts:266 does not consume it. A disconnect during xctestrun build/launch can therefore continue and retain a runner/lease after the global abort fallback was removed. Add a blocked-startup disconnect regression proving only that request's prep/session is stopped or never retained.
P2 — targeted close failures are misreported as cleanup failures. src/daemon/handlers/session-close.ts wraps the actual platform close dispatch in attemptCleanup, then records Closed before throwing the aggregate. This loses the original AppError details/hint and can log success for a failed close. Preserve the original close failure semantics while still attempting subsequent resource cleanup, with a regression for a distinct thrown AppError.

CI is green, but this head is not ready.

devin-ai-integration · 2026-07-12T10:54:46Z

@thymikee both blockers addressed at 9647092:

1. Request cancellation now covers blocked runner startup/build/launch, no global abort. startRunnerSessionWithLease pulls getRequestSignal(options.requestId) and threads it through ensureXctestrunArtifact → buildRunnerXctestrun → runCmdStreaming({ signal }) (the xcodebuild build-for-testing) and into the runCmdBackground({ signal }) runner launch — both killProcessTree on abort, so a disconnected request's prep is actually terminated. It stays request-scoped: getRequestSignal returns only that request's controller, an unrelated concurrent build on another device is untouched, and an internal caller (daemon shutdown) gets undefined. To guarantee a canceled prep is never retained, handlePrepareHealthFailure now invalidates the just-created session on a request-canceled error before rethrowing. abortAllIosRunnerSessions remains internal-only for deliberate shutdown. New test (runner-client.test.ts): two concurrent builds on different devices/signals — aborting A kills A's build while B's signal stays unaborted and B completes; asserts each build got its own signal.

2. Targeted platform-close error is preserved. The close dispatch is no longer wrapped in attemptCleanup; dispatchTargetedPlatformClose captures its AppError and it is rethrown with original code/details/hint. Closed is recorded only when the close succeeded, and subsequent independent cleanup (apple_runner stop, runtime hints, materialized paths) + lease release + session delete still run on failure. New regression (session-close-shutdown.test.ts) asserts the exact AppError is rethrown, no Closed action is recorded, subsequent cleanup ran, and the session was deleted. (handleCloseCommand was split into focused helpers to stay under the complexity gate.)

Rebased onto latest main (incl. #1224, clean). Checks: format:check, lint, typecheck, check:layering, fallow pass; focused suites (165 tests) + both integration files (AGENT_DEVICE_REQUIRE_LOOPBACK_TESTS=1) pass. check:affected build (tsdown) still fails only on the pre-existing environmental missing-unrun peer dep (identical on clean main); CI's build is authoritative.

devin-ai-integration · 2026-07-12T11:07:17Z

+  const { req, session, logPath, attemptCleanup } = params;
+  if (!shouldDispatchPlatformClose(req, session)) return undefined;
+  if (shouldStopAppleRunnerBeforeTargetedClose(session)) {
+    await attemptCleanup('apple_runner_pre_close', () => stopAppleRunnerForClose(session));


P2 (dependency concern): making the pre-close runner stop best-effort changes non-simulator close semantics — a failed pre-close stop now proceeds with the platform close.

On origin/main this same stopAppleRunnerForClose call was an un-wrapped await inside the close block, so a throw skipped dispatchCommand('close') entirely (propagated out via the try/finally, close never dispatched). Wrapping it in attemptCleanup swallows the error into cleanupFailures and falls straight through to the try { await dispatchCommand(... 'close' ...) } below.

The stop is intentionally ordered before dispatch "to avoid runner/app races" (per the comment in stopOrRetainAppleRunnerAfterClose and shouldStopAppleRunnerBeforeTargetedClose = apple && !iosSimulator, i.e. real device / macOS). So a failed pre-close stop now dispatches close while the runner may still be alive — the exact race the ordering guards against. The post-close stopOrRetainAppleRunnerAfterClose re-attempt only mitigates after the racy close.

Also note: if the pre-close stop fails but the close then succeeds, platformCloseError is undefined, so Closed is recorded and the cleanup aggregate (containing apple_runner_pre_close) is thrown — a Closed-and-error result.

Suggest either keeping the pre-close stop error-propagating for non-simulator targets (skip the close as before, letting the close error path own it), or, if proceeding is intentional, documenting why it's safe and adding a non-simulator regression. No test covers this path — the Blocker-2 test uses kind: 'simulator', so shouldStopAppleRunnerBeforeTargetedClose is false and the pre-close branch never runs.

Fixed in 4d729f0. The pre-close runner stop for non-simulator Apple targets is no longer best-effort. dispatchTargetedPlatformClose now awaits stopAppleRunnerForClose(session) directly and, on failure, returns that error and skips the close dispatch entirely — preserving the original code/details/hint and never recording Closed. Later independent cleanup (stopOrRetainAppleRunnerAfterClose, runtime hints, materialized paths) still runs, and lease release + session delete are unchanged. This also resolves the Closed-and-error case you flagged: a failed pre-close stop now sets platformCloseError, so Closed is never recorded and the original pre-close error is what's rethrown.

Added a real-device-shaped regression in session-close-shutdown.test.ts (kind: 'device', so shouldStopAppleRunnerBeforeTargetedClose is true): pre-close stop rejects → dispatchCommand is never called, the original AppError code/details/hint are preserved, no Closed action is recorded, later cleanup still re-attempts the runner stop, and the session is still deleted.

devin-ai-integration · 2026-07-12T11:07:33Z

 }): Promise<PrepareAttemptResult> {
  const { device, session, command, options, signal, attempt, error } = params;
  const appErr = asAppError(error, 'COMMAND_FAILED');
+  if (isRequestCanceledError(appErr)) {


P2 (Blocker 1 — proof gap, not a correctness bug): the runtime wiring is correct by code, but the new runner-client.test.ts test proves only the artifact-level signal threading and leaves the parts the blocker actually asks to prove untested.

What I verified by reading (all correct):

disconnect → markRequestCanceled(requestId) aborts the controller registered by registerRequestAbort at request start (http-server / transport);

requestId flows meta → dispatch ctx (interactions.ts:78) → runner options → startRunnerSessionWithLease getRequestSignal(options.requestId);

signal → ensureXctestrunArtifact → buildRunnerXctestrun → runCmdStreaming({signal}) and → launch runCmdBackground({signal}), both killProcessTree on abort (watchCommandAbort);

retention: a build abort throws before runnerSessions.set/writeRunnerLease (no session/lease); a launch abort (allowFailure:true) stores the session+lease, then the health check runs with the aborted signal, the transport maps signal.aborted → createRequestCanceledError(), and this branch invalidates it via invalidateRunnerSessionBestEffort (removes the stored session + releases the lease).

What no test exercises:

the request-registry resolution — getRequestSignal(options.requestId) in startRunnerSessionWithLease (the test passes a signal directly to ensureXctestrunArtifact, bypassing the registry), so an actual disconnect's requestId→controller reaching startup is unproven;

the launch runCmdBackground({signal}) cancellation;

this session/lease-retention branch (handlePrepareHealthFailure request-canceled → invalidate) — the PR's core "never retained" guarantee has zero coverage.

Suggest one test at prepareLocalIosRunner/ensureRunnerSession level driven through the registry: registerRequestAbort(reqA), start prep for reqA, abort mid-launch, assert reqA's session/lease is stopped/not retained while a concurrent reqB on a different device keeps its session. That would actually prove the blocker rather than the artifact seam.

Fixed in 4d729f0. Added an integration-level test in runner-session.test.ts driven through the request registry rather than the artifact seam: prepareLocalIosRunner cancellation stops only the disconnected request and preserves unrelated concurrent prep.

It uses registerRequestAbort(reqA)/registerRequestAbort(reqB) and exercises the real prepareLocalIosRunner → ensureRunnerSession → startRunnerSessionWithLease path (no artifact-level mock). Request B prepares normally and its session is retained. Request A is canceled mid-startup: the launch stores the session + lease, then the health check runs with A's now-aborted signal and rejects with a request-canceled error, driving the handlePrepareHealthFailure request-canceled branch → invalidateRunnerSessionBestEffort (this branch). Assertions:

A's runCmdBackground launch received A's request signal, and that signal is aborted (a real launch would be killed via the process tree).

getRunnerSessionSnapshot(deviceA) is null — A's session/lease is not retained.

getRunnerSessionSnapshot(deviceB) is still alive — the unrelated request survives.

thymikee · 2026-07-12T12:37:44Z

Re-review at 4d729f0: the prior runner-retention and targeted-close blockers are fixed. One cancellation-propagation issue remains.

In runner-artifact.ts, runCmdStreaming receives the request signal and aborts with reason request_canceled, but the catch at the build-for-testing boundary wraps every error as a new generic COMMAND_FAILED and nests the original details. isRequestCanceledError can no longer recognize it, so explicit cancellation during runner build is reported as an xcodebuild failure with a cache/build hint.

Preserve the top-level request-canceled semantics before wrapping, and add a focused abort-during-build regression. The ready-for-human label is being removed pending that fix. No fixer dispatched.

devin-ai-integration · 2026-07-12T12:42:03Z

Fixed at a9752f22a: the build boundary now rethrows request-canceled errors unchanged before generic xcodebuild wrapping, and the concurrent build regression asserts exact error identity plus isRequestCanceledError. Rebased onto current main; focused 154 tests and deterministic checks pass.

thymikee · 2026-07-13T08:56:42Z

Coordinator re-review at a9752f22a: one P1 blocker remains.

A normal runner command canceled during first startup can still strand session/lease ownership. startRunnerSessionWithLease passes the request signal to runCmdBackground, then unconditionally builds/writes the lease and stores a ready: false session. In the normal executeRunnerCommand path, a typed cancellation is rethrown without invalidating that newly created session; the new cancellation invalidation exists only in handlePrepareHealthFailure for the explicit prepare path.

Please make cancellation after launch request-scoped and ensure the just-created session/lease is never retained, then add a normal command regression that cancels during startup/launch and proves the canceled request leaves no session/lease while an unrelated request remains alive.

Secondary isolation gap: concurrent requests that reuse the same supplied meta.requestId overwrite the single controller in requestAbortControllers. Either enforce/document in-flight uniqueness at the transport boundary or make tracking collision-safe and cover it.

Also tighten the pre-header HTTP regression: its helper currently resolves immediately after req.destroy(), so the test itself should explicitly await the server-side abort observation rather than relying on the second test to cover that path.

ready-for-human remains withheld.

Cancel HTTP requests that lose their client before response headers, and scope disconnect cancellation to the affected request/device/session instead of a global Apple runner abort. Make session resource teardown failure-isolated so one rejected step no longer skips later cleanup, while preserving lease release and session deletion. Closes #1220 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

…se error preservation Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

…gate Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

… prep-cancellation coverage Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

devin-ai-integration · 2026-07-13T09:06:55Z

Addressed the coordinator re-review from a9752f22a at 2f2582e7a: normal commands now dispose an already-aborted launch before lease/session persistence and invalidate any unready session canceled during initial readiness; the focused regression covers both timing windows and proves only the unrelated survivor lease/session remains. Concurrent duplicate request IDs are rejected with INVALID_ARGS/duplicate_request_id without replacing or clearing the original registration, and the pre-header HTTP test now awaits explicit server-side abort observation. Rebased onto current main; check:affected --run passed (4,090 unit tests + 124 provider-integration tests), along with format, lint, typecheck, layering, Fallow, build, and focused cancellation suites.

thymikee · 2026-07-13T09:45:56Z

Coordinator re-review at 2f2582e7a: all three prior source/test findings are fixed and all 22 checks pass. The normal-command launch/readiness paths now dispose canceled startup state before lease retention, duplicate active request IDs are rejected without stealing cleanup ownership, and the pre-header regression waits for server-side abort observation.

One readiness evidence gap remains under the repository device-facing checklist: the new runner cancellation tests mock process launch/readiness, and the iOS smoke does not disconnect a real client during fresh XCTest startup. Please provide one practical simulator run that disconnects during fresh runner build/launch and shows the canceled runner process/session/lease is gone while an unrelated request remains usable. No code change is requested unless that run fails. ready-for-human remains withheld pending that evidence.

thymikee · 2026-07-13T14:46:31Z

Simulator evidence for the remaining readiness gap (PR head 2f2582e7a):

Started a fresh XCTest runner on iPhone 17 simulator (D74E0B66-57EB-4EC1-92DC-DA0A30581FE7) with a forced clean/rebuild.
Disconnected only the request socket immediately after ios_runner_startup_launch_xcodebuild, before readiness (ready:false).
The request failed with request canceled; diagnostics recorded ios_runner_session_invalidated with reason prepare_runner_request_canceled.
The launched xcodebuild test-without-building emitted an exit event, no matching runner process remained, and the runner lease directory was empty.
Through the same daemon, an unrelated iPhone 16 simulator request opened successfully and produced a healthy XCTest snapshot (nodeCount: 50, snapshotQuality.state: healthy, backend tree).

All sessions and temporary daemon state were closed/cleaned. No code changes were needed.

github-actions · 2026-07-13T14:51:00Z

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-13 14:50 UTC

Agent-supervised re-record repair lifecycle, rebased onto #1225's failure-isolated close teardown. Consolidated from the earlier iterative rounds into the final teardown-commits model: - R7 keep-alive keyed off PERSISTED transaction state (repairSessionHeld signal), so a `replay --from` continuation without --save-script is still held on divergence. - Commit gated on transaction COMPLETION (saveScriptComplete/saveScriptCommitted), never on `close` alone — no prefix is ever published. - Single commit path: `commitRepairBeforeClose` runs before #1225's `runSessionCloseTeardown` destructive steps; a repair-armed session skips the teardown's ordinary writeSessionLog, non-repair keeps it. Idle-reap/shutdown commit-on-completion or tombstone via `finalizeRepairTeardown`. - BLOCKER fixes: reaped `replay --from` -> REPAIR_SESSION_EXPIRED; commit failures surfaced (not swallowed) and keep the session for retry (with healed-path reporting on success); race-safe atomic no-clobber publish; minimal `[open, close]` arms the transaction. Integrated with #1225: keeps runSessionCloseTeardown's failure-isolated cleanup + preserved platform-close error; repair commit happens first so a failed commit keeps the session addressable.

thymikee self-assigned this Jul 12, 2026

devin-ai-integration Bot force-pushed the devin/1783850865-isolate-disconnect-cleanup branch from 3f0f7c6 to 9647092 Compare July 12, 2026 10:53

devin-ai-integration Bot commented Jul 12, 2026

View reviewed changes

devin-ai-integration Bot added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 12, 2026

thymikee removed the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 12, 2026

devin-ai-integration Bot force-pushed the devin/1783850865-isolate-disconnect-cleanup branch from 4d729f0 to a9752f2 Compare July 12, 2026 12:41

devin-ai-integration Bot added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 12, 2026

thymikee removed the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 13, 2026

thymikee and others added 7 commits July 13, 2026 09:03

fix(daemon,runner): request-scoped prep cancellation and platform-clo…

63d5238

…se error preservation Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

refactor(daemon): split session-close teardown to satisfy complexity …

2e528a5

…gate Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

fix(daemon,runner): require pre-close runner stop and add integration…

03af5b2

… prep-cancellation coverage Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

fix(apple): preserve request cancellation during runner build

a77975b

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

fix(daemon): close request cancellation isolation gaps

d5f8aa2

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

refactor(request): keep cancellation cleanup owned

2f2582e

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

devin-ai-integration Bot force-pushed the devin/1783850865-isolate-disconnect-cleanup branch from a9752f2 to 2f2582e Compare July 13, 2026 09:06

devin-ai-integration Bot added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 13, 2026

thymikee removed the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 13, 2026

devin-ai-integration Bot added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 13, 2026

thymikee merged commit ea3813d into main Jul 13, 2026
22 checks passed

thymikee deleted the devin/1783850865-isolate-disconnect-cleanup branch July 13, 2026 14:50

Uh oh!

Conversation

devin-ai-integration Bot commented Jul 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

devin-ai-integration Bot commented Jul 12, 2026

🤖 Devin AI Engineer

Uh oh!

github-actions Bot commented Jul 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Size Report

Uh oh!

devin-ai-integration Bot commented Jul 12, 2026

Uh oh!

thymikee commented Jul 12, 2026

Uh oh!

devin-ai-integration Bot commented Jul 12, 2026

Uh oh!

devin-ai-integration Bot Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jul 12, 2026

Choose a reason for hiding this comment

Uh oh!

thymikee commented Jul 12, 2026

Uh oh!

devin-ai-integration Bot commented Jul 12, 2026

Uh oh!

thymikee commented Jul 13, 2026

Uh oh!

devin-ai-integration Bot commented Jul 13, 2026

Uh oh!

thymikee commented Jul 13, 2026

Uh oh!

thymikee commented Jul 13, 2026

Uh oh!

Uh oh!

github-actions Bot commented Jul 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

devin-ai-integration Bot commented Jul 12, 2026 •

edited

Loading

github-actions Bot commented Jul 12, 2026 •

edited

Loading